Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #416, uiTableModelRowInserted on windows adds 2 rows instead of 1. #484

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pftbest
Copy link

@pftbest pftbest commented Feb 5, 2020

When LVM_INSERTITEM is called after we updated LVM_SETITEMCOUNT, the actual count will be increased by one causing one extra item to appear in the table and loading data from the model.
This extra item is invisible at first because it doesn't get painted but you can click on it to confirm that it is actually there.

To fix the problem we need to call INSERTITEM first and then update the ITEMCOUNT.

Fixes #416
Fixes andlabs/ui#369

cc @bcampbell
r? @andlabs

…ad of 1.

When LVM_INSERTITEM is called after we updated LVM_SETITEMCOUNT,
the actual count will be increased by one causing one extra item
to appear in the table and loading data from the model.
This extra item is invisible at first because it doesn't get painted but
you can click on it to confirm that it is actually there.

To fix this problem we need to call INSERTITEM first and
then update the ITEMCOUNT.

Fixes andlabs#416
Fixes andlabs/ui#369

cc @bcampbell
r? @andlabs
@szanni
Copy link

szanni commented Sep 20, 2020

Patch looks good to me.

The MS docs don't mention anything about setting an item count on insertion.
Looking (again) at the WINE sources however tells me the list view increases its internal counter on LVM_INSERTITEM with LVS_OWNERDATA (which libUI uses).
Debug printing the item count before and after insertion confirms this.

My only question would be: why not remove the call to LVM_SETITEMCOUNT entirely? It is clearly redundant.

@andlabs
Copy link
Owner

andlabs commented Sep 27, 2020

Yeah, I tried looking up why I did this, and couldn't find anything. "Update selection state" is a very vague comment in hindsight... I guess you can try removing the call and seeing what happens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

table error on windows10 uiTableModelInsertRow() causes invalid rows to be accessed (on windows)
4 participants